fix(migrate-ts): refuse a primary-key move instead of silently dropping the PK (#258) by dmealing · Pull Request #262 · metaobjectsdev/metaobjects

dmealing · 2026-08-02T16:07:02Z

Intent

Branch fix/258-migrate-pk-detect-refuse implements GitHub issue #258 (npm-only, migrate-ts + cli), for the coordinated release (npm gets this on top of #246/#259; other registries unaffected — schema/migrate is TS-owned, ADR-0015).

THE BUG: adopting an existing database whose PRIMARY KEY differs from the metadata identity had no expressible migration. The diff/emit has NO primary-key change kind, so a moved PK (e.g. live 'PRIMARY KEY (user_id)', metadata identity 'id' uuid) degraded SILENTLY into an add-column 'id' + drop-column 'user_id': the old PK column and its constraint were dropped, the new column was never made PK, leaving the table with NO primary key, so every foreign key referencing it failed at apply ('there is no unique constraint matching given keys'). Only observable when adopting an existing DB (--from-db) whose PK disagrees with metadata; greenfield create carries its PK inline. Follow-on from #255.

THE FIX (detect-and-refuse; the #226->#241 arc precedent, chosen deliberately over auto-migrate which is a later follow-up): migration generation now compares the introspected primary key to the metadata identity and throws PrimaryKeyChangeError (new error class in migrate-ts errors.ts, exported) with a clear message naming the table + both PKs, INSTEAD of emitting the un-appliable SQL. Placed AFTER rename detection (detectColumnRenames) so a PK column that was merely RENAMED (the engine preserves the PK through RENAME COLUMN) is NOT mistaken for a move — the check maps live PK names through detected rename-column changes before comparing. Gated by a new DiffArgs.refusePrimaryKeyChange flag (off by default -> existing diff callers and the read-only drift/verify path are byte-identical; verify still REPORTS drift rather than throwing). The flag is set ONLY by the two migration-generation paths: the online 'meta migrate --db' diff call and the offline planOffline. The CLI (migrate.ts) catches PrimaryKeyChangeError at both throw sites and emits a clean structured error + exit 1.

INTENT NOTES: refusing (not auto-migrating) is the deliberate chosen approach; the read-only verify path intentionally does NOT refuse; byte-identity for every migration that is NOT a PK move is the guardrail (full migrate-ts suite 670 pass unchanged). Verified: 5 unit tests (refuse on a move; no-refuse on unchanged PK; no-refuse on a resolved PK-column rename; no-throw without the flag) + a real-Postgres integration test (gated on MIGRATE_TS_PG_URL) that creates a live user_profiles PK(user_id) with a referencing FK, introspects it, and asserts the refusal fires on the genuine reproduction (I ran it against a throwaway postgres:16 container — green). migrate-ts + cli typecheck clean; cli suite 416 pass. Any real-PG integration failures in the full suite (inet/@lenient, array value-semantics, CHECK comma-spacing) are pre-existing postgres-version golden mismatches, NOT this change — my new code is behind the refusePrimaryKeyChange guard which those tests never set.

What Changed

meta migrate's diff/emit now compares the introspected primary key against the metadata identity and throws a new exported PrimaryKeyChangeError (migrate-ts) when they disagree, instead of degrading a moved PK into an add/drop-column pair that silently dropped the PK — gated by a new refusePrimaryKeyChange DiffArgs flag set by all three generation paths (online --db, offline planOffline, and the D1 path).
The check runs after rename detection so a renamed PK column is mapped through detected rename changes before comparing and is not mistaken for a move; the read-only verify path is unaffected (reports drift, doesn't throw), so every migration that is not a PK move stays byte-identical.
The CLI catches PrimaryKeyChangeError at all throw sites and emits a structured error + exit 1; the CHANGELOG, the bug doc, and the migrate-and-drift guide are updated.

Risk Assessment

✅ Low: The incremental commit correctly and minimally extends the existing detect-and-refuse guard to the D1 generation path, exactly mirroring the two already-shipped handlers (same flag, same catch placement before the ambiguous branch, identical message/hint/exit code), with the supporting details verified (import, scope, clean _fmt→fmt rename, byte-identity guardrail intact) and no new code paths or behavioral ambiguity introduced.

Testing

Stood up the genuine reproduction (a live table with PRIMARY KEY(user_id) plus a referencing FK, against metadata identity id uuid) in a throwaway postgres:16-alpine container, then drove the real meta migrate CLI: it refuses with exit 1 and a clean structured error naming the table and both PKs, while meta verify --db on the same DB reports drift without refusing — confirming the guard is migrate-only. The 5 new unit tests, the real-PG integration test, the migrate-ts suite (670/0), and the cli suite (416/0) all pass, with the working tree left clean and the container torn down.

Evidence: CLI refusal + verify contrast transcript

meta migrate --from-db (JSON, EXIT 1): {"error":"migrate: primary key of "user_profiles" differs from the live database: live PRIMARY KEY (user_id) vs metadata PRIMARY KEY (id). migrate cannot express a primary-key change (there is no add/drop-primary-key change kind), so this would silently drop the constraint and break every foreign key that references this table. Align the primary key manually — or reconcile the metadata identity to match the live table — before migrating.","hint":"align the primary key manually, or reconcile the metadata identity to match the live table"} meta verify --db (same DB, EXIT 1 = drift REPORTED, not refused; refusal-message grep = 0): meta: meta verify — schema drift vs postgres://...:5444/mo_pk258 (3 change(s)): meta: - table agent_configs meta: + column user_profiles.id meta: - column user_profiles.user_id

================================================================================
#258 — meta migrate REFUSES a primary-key move (online --from-db path), while the
read-only `meta verify --db` REPORTS the same drift without refusing.

Reproduction (throwaway postgres:16-alpine on :5444, db mo_pk258):
  LIVE DB  : CREATE TABLE user_profiles (user_id bigint PRIMARY KEY, auth_user_id text NOT NULL);
             CREATE TABLE agent_configs (id bigint PRIMARY KEY,
               created_by bigint NOT NULL REFERENCES user_profiles(user_id));
  METADATA : object.entity UserProfile -> table user_profiles, identity.primary @fields [id] (uuid)
  => live PK(user_id) DISAGREES with metadata identity(id): the #258 PK-move bug.

================================================================================
[1] meta migrate --from-db --db <URL> --dialect postgres --slug pk258   (JSON)
================================================================================
EXIT_CODE=1
--- STDOUT (--format json) ---
{
  "error": "migrate: primary key of \"user_profiles\" differs from the live database: live PRIMARY KEY (user_id) vs metadata PRIMARY KEY (id). migrate cannot express a primary-key change (there is no add/drop-primary-key change kind), so this would silently drop the constraint and break every foreign key that references this table. Align the primary key manually — or reconcile the metadata identity to match the live table — before migrating.",
  "hint": "align the primary key manually, or reconcile the metadata identity to match the live table"
}

--- STDERR ---
meta: migrate: primary key of "user_profiles" differs from the live database: live PRIMARY KEY (user_id) vs metadata PRIMARY KEY (id). migrate cannot express a primary-key change (there is no add/drop-primary-key change kind), so this would silently drop the constraint and break every foreign key that references this table. Align the primary key manually — or reconcile the metadata identity to match the live table — before migrating.

================================================================================
[2] meta migrate --from-db --db <URL> --dialect postgres --slug pk258   (default text)
================================================================================
EXIT_CODE=1
--- STDOUT (text) ---
error: "migrate: primary key of \"user_profiles\" differs from the live database: live PRIMARY KEY (user_id) vs metadata PRIMARY KEY (id). migrate cannot express a primary-key change (there is no add/drop-primary-key change kind), so this would silently drop the constraint and break every foreign key that references this table. Align the primary key manually — or reconcile the metadata identity to match the live table — before migrating."
hint: "align the primary key manually, or reconcile the metadata identity to match the live table"

--- STDERR ---
meta: migrate: primary key of "user_profiles" differs from the live database: live PRIMARY KEY (user_id) vs metadata PRIMARY KEY (id). migrate cannot express a primary-key change (there is no add/drop-primary-key change kind), so this would silently drop the constraint and break every foreign key that references this table. Align the primary key manually — or reconcile the metadata identity to match the live table — before migrating.

================================================================================
[3] meta verify --db <URL>   (read-only — MUST report drift, MUST NOT refuse)
================================================================================
EXIT_CODE=1 (drift found, reported — NOT a refusal)
--- STDERR ---
meta: meta verify — schema drift vs postgres://postgres:***@localhost:5444/mo_pk258 (3 change(s)):
meta:   - table agent_configs
meta:   + column user_profiles.id
meta:   - column user_profiles.user_id

Refusal-message grep count (must be 0):
  verify_out.txt:0  verify_err.txt:0

Evidence: Targeted test-run results (unit + real-PG + guardrails)

#258 migrate PK-move detect-and-refuse — targeted test results
(spawned a throwaway postgres:16-alpine on :5444 for the PG-gated run; now torn down)

[1] Unit: migrate-ts/test/diff-primary-key-refuse.test.ts   (no PG)
    bun test packages/migrate-ts/test/diff-primary-key-refuse.test.ts
    => 5 pass / 0 fail
       - REFUSES when live PK differs from metadata identity (add/drop, not rename)
       - refusal names the table + both PKs
       - does NOT refuse when PK unchanged
       - does NOT refuse a PK-column RENAME (engine preserves PK through RENAME COLUMN)
       - WITHOUT the flag: diff does not throw (verify/drift path unchanged)

[2] Real-Postgres integration: migrate-ts/test/integration/pg-primary-key-refuse-258.test.ts
    MIGRATE_TS_PG_URL=postgres://...:5444/mo_pk258 bun test .../pg-primary-key-refuse-258.test.ts
    => 1 pass / 0 fail
       Proves real introspection reads the live PK(user_id); WITHOUT guard the diff
       silently degrades to add-column id + drop-column user_id; WITH guard it throws
       PrimaryKeyChangeError naming user_profiles / [user_id] / [id].

[3] Byte-identity guardrail: full migrate-ts suite (PG unset -> pg tests skip)
    env -u MIGRATE_TS_PG_URL bun test packages/migrate-ts
    => 670 pass / 18 skip / 0 fail   (matches intent: "670 pass unchanged")

[4] CLI suite: bun test packages/cli
    => 416 pass / 2 skip / 0 fail    (matches intent: "cli suite 416 pass")

End-user CLI behavior is captured in cli-pk258-refuse-transcript.txt:
    meta migrate --from-db ... => exit 1, structured error naming user_profiles + both PKs
    meta verify    --db     ... => reports 3-change drift, does NOT refuse

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed ✅

⚠️ server/typescript/packages/cli/src/commands/migrate.ts:1024 - The D1 migrate path (runD1Migrate) is a third migration-generation path that calls diff() at migrate.ts:1024 WITHOUT refusePrimaryKeyChange: true, and its catch block (migrate.ts:1040-1050) does NOT catch PrimaryKeyChangeError (it only handles the 'aborted by onAmbiguous' case then re-throws). The intent names 'only two generation paths' (online --db and offline planOffline), but D1 generate is a distinct generation path over the same shared diff/emit pipeline. The bug migrate: no primary-key change kind, so moving a table's PK leaves it with none and every referencing FK fails #258 ('migrate has no primary-key change kind') is engine-wide, not Postgres-specific: adopting an existing D1 DB whose live PK differs from the metadata identity (e.g. live PRIMARY KEY (user_id), metadata identity id) still degrades in the diff layer into add-column(id) + drop-column(user_id). On modern D1 (SQLite >= 3.35) SUPPORTS_DROP is true, so the sqlite emitter emits a bare ALTER TABLE ... DROP COLUMN user_id (sqlite.ts:222) rather than the recreate-and-copy rebuild that would re-establish the expected PK from newTable.primaryKey (sqlite.ts:288). So the user gets no clean generation-time refusal on D1 — at apply the migration either silently drops the PK (the exact data-integrity failure migrate: no primary-key change kind, so moving a table's PK leaves it with none and every referencing FK fails #258 fixes for Postgres) or errors opaquely. The same authorized failure remains reachable via this path. Recommend confirming whether D1 was intentionally excluded; if not, the earliest supported shared boundary is to set refusePrimaryKeyChange: true in the D1 diff call (migrate.ts:1024) and catch PrimaryKeyChangeError in its catch block (mirroring the online/offline handlers at migrate.ts:405-411 / 824-829). This does not violate the byte-identity guardrail — the flag only throws on an actual PK move; every other D1 migration is byte-identical.
ℹ️ server/typescript/packages/migrate-ts/src/diff/index.ts:320 - The PK comparison is order-sensitive: livePk.every((col, i) => col === wantPk[i]). A live composite PK with the same columns as the metadata identity but in a different order (e.g. live (a,b) vs metadata @fields [b,a]) — plausible when adopting a hand-built DB whose column order differs from the author's identity declaration — would be refused as a 'move', even though the uniqueness constraint is functionally identical. The refusal is safe (no data loss, clear message), so this is a conservative UX friction, not a defect. Making it set-based (order-insensitive) is a one-line behavioral change but a deliberate product decision the author should own.

🔧 Fix: Guard D1 migrate path against primary-key moves (#258)
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

bun test packages/migrate-ts/test/diff-primary-key-refuse.test.ts -> 5 pass / 0 fail (refuse-on-move, message-names-table+both-PKs, no-refuse-unchanged, no-refuse-on-PK-column-rename, no-throw-without-flag)
MIGRATE_TS_PG_URL=postgres://postgres:test@localhost:5444/mo_pk258 bun test packages/migrate-ts/test/integration/pg-primary-key-refuse-258.test.ts -> 1 pass / 0 fail (real PG introspection reads live PK(user_id); unguarded diff = add id + drop user_id; guarded diff throws PrimaryKeyChangeError)
CLI end-to-end (throwaway PG, live user_profiles PK(user_id) + referencing FK; metadata identity id uuid): meta migrate --cwd demo --format json --from-db --db $DB --dialect postgres --slug pk258 -> EXIT 1, structured JSON error naming user_profiles + both PKs
CLI end-to-end default text format: same command without --format -> EXIT 1, error/hint block on stdout + human message on stderr
Read-only path contrast: meta verify --cwd demo --db $DB -> EXIT 1 reporting 3-change drift (drop agent_configs, + user_profiles.id, - user_profiles.user_id); grep for refusal message = 0 matches (does NOT refuse)
env -u MIGRATE_TS_PG_URL bun test packages/migrate-ts -> 670 pass / 18 skip / 0 fail (byte-identity guardrail; matches intent '670 pass unchanged')
env -u MIGRATE_TS_PG_URL bun test packages/cli -> 416 pass / 2 skip / 0 fail (migrate.ts PrimaryKeyChangeError catch sites; matches intent 'cli suite 416 pass')

🔧 **Document** - 1 issue found → auto-fixed ✅

ℹ️ docs/features/migrations-and-drift.md:66 - The migrate: no primary-key change kind, so moving a table's PK leaves it with none and every referencing FK fails #258 primary-key-move refusal is a new user-facing meta migrate behavior (a user adopting an existing DB whose PK disagrees with metadata now gets a clear error + exit 1 instead of a silent broken migration). Its authoritative record is now accurate in two owners — the bug doc (RESOLVED) and the CHANGELOG ([Unreleased]). The always-loaded migrate guide, docs/features/migrations-and-drift.md, does not mention it. That guide already documents an analogous refusal family (the 'D1: rebuilding a foreign-key-referenced table' section covering the migrate --dialect d1: the sqlite table-rebuild's PRAGMA foreign_keys OFF is a no-op on remote D1, so any parent-table rebuild fails with FOREIGN KEY constraint failed #226->D1: auto-migrate referenced-table rebuilds via FK cascade (follow-up to #226) #241 detect/cascade arc, and the multi-table-FK-cycle refusal), so a short pointer there is arguably warranted for an adopting-DB user who hits the error. I did not add it because (a) the guide does not enumerate refusals exhaustively (data-migration refusal, ambiguous-rename abort, etc. are also absent), so this is a gap rather than a contradiction this change created, and (b) adding prose there would duplicate the bug-doc/CHANGELOG fact against the 'never synchronize prose copies' rule. Flagging as a placement judgment call: confirm the two owners suffice, or add a one-paragraph PK-move-refusal note to migrations-and-drift.md.

🔧 Fix: Document #258 PK-move refusal in migrate guide
✅ Re-checked - no issues remain.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

…pping the PK Adopting a database whose PRIMARY KEY differs from the metadata identity had no expressible migration: the diff/emit has no primary-key change kind, so a moved PK (e.g. live PRIMARY KEY (user_id), metadata identity id) degraded into an add-column + drop-column — the old PK column and its constraint were dropped, the new column was never made PK, and the table was left with NO primary key, so every referencing foreign key failed at apply. Detect-and-refuse (the #226->#241 arc precedent): migration generation now compares the introspected primary key to the metadata identity and throws PrimaryKeyChangeError with a clear message instead of emitting the un-appliable SQL. Runs after rename detection, so a PK column that was merely RENAMED (the engine preserves the PK through RENAME COLUMN) is not mistaken for a move. Gated by a new DiffArgs.refusePrimaryKeyChange flag set only by the two migration-generation paths (online meta migrate --db and offline planOffline); the read-only drift/verify path is unchanged, so meta verify still reports drift. Auto-migrating a PK move (add/drop-primary-key change kinds) is a later follow-up. npm-only (migrate-ts + cli). Gated by unit tests (refuse on a move, not on an unchanged PK, not on a renamed PK column, and off by default) plus a real-Postgres integration test proving introspection reads the live PK and the refusal fires on the genuine reproduction. Existing meta gen / meta migrate output is byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr

…258)

…ug doc

dmealing and others added 4 commits August 2, 2026 11:00

no-mistakes(review): Guard D1 migrate path against primary-key moves (#…

737d324

…258)

no-mistakes(document): Document #258 PK-refuse fix in CHANGELOG and b…

e0deec4

…ug doc

no-mistakes(document): Document #258 PK-move refusal in migrate guide

cccbc61

dmealing merged commit aa29d70 into main Aug 2, 2026
1 check passed

dmealing deleted the fix/258-migrate-pk-detect-refuse branch August 2, 2026 16:08

dmealing mentioned this pull request Aug 4, 2026

migrate: no primary-key change kind, so moving a table's PK leaves it with none and every referencing FK fails #258

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(migrate-ts): refuse a primary-key move instead of silently dropping the PK (#258) - #262

fix(migrate-ts): refuse a primary-key move instead of silently dropping the PK (#258)#262
dmealing merged 4 commits into
mainfrom
fix/258-migrate-pk-detect-refuse

dmealing commented Aug 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

dmealing commented Aug 2, 2026

Intent

What Changed

Risk Assessment

Testing

Pipeline

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant